-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Sync footstep audio #2177
Sync footstep audio #2177
Conversation
…nd(TYSM for the help Jannify) Co-authored-by: Jannify <[email protected]>
NitroxClient/Communication/Packets/Processors/FootstepPacketProcessor.cs
Outdated
Show resolved
Hide resolved
NitroxClient/Communication/Packets/Processors/FootstepPacketProcessor.cs
Outdated
Show resolved
Hide resolved
NitroxClient/Communication/Packets/Processors/FootstepPacketProcessor.cs
Outdated
Show resolved
Hide resolved
NitroxClient/Communication/Packets/Processors/FootstepPacketProcessor.cs
Outdated
Show resolved
Hide resolved
NitroxClient/Communication/Packets/Processors/FootstepPacketProcessor.cs
Outdated
Show resolved
Hide resolved
NitroxServer/Communication/Packets/Processors/FootstepPacketProcessor.cs
Outdated
Show resolved
Hide resolved
NitroxServer/Communication/Packets/Processors/FootstepPacketProcessor.cs
Outdated
Show resolved
Hide resolved
NitroxServer/Communication/Packets/Processors/FootstepPacketProcessor.cs
Outdated
Show resolved
Hide resolved
NitroxServer/Communication/Packets/Processors/FootstepPacketProcessor.cs
Outdated
Show resolved
Hide resolved
NitroxServer/Communication/Packets/Processors/FootstepPacketProcessor.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Jannify <[email protected]>
Co-authored-by: Jannify <[email protected]>
Co-authored-by: Jannify <[email protected]>
…implement fetching the value from .csv file
Did another test with the help of kookoo, CSV sound range works, tested everything else and it was still fine so review away |
NitroxClient/Communication/Packets/Processors/FootstepPacketProcessor.cs
Outdated
Show resolved
Hide resolved
NitroxServer/Communication/Packets/Processors/FootstepPacketProcessor.cs
Outdated
Show resolved
Hide resolved
NitroxServer/Communication/Packets/Processors/FootstepPacketProcessor.cs
Outdated
Show resolved
Hide resolved
NitroxServer/Communication/Packets/Processors/FootstepPacketProcessor.cs
Outdated
Show resolved
Hide resolved
NitroxServer/Communication/Packets/Processors/FootstepPacketProcessor.cs
Outdated
Show resolved
Hide resolved
NitroxServer/Communication/Packets/Processors/FootstepPacketProcessor.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Jannify <[email protected]>
Co-authored-by: Jannify <[email protected]>
Will do another test after approving reviews, so don't merge until that happens(will post here again when it does) |
NitroxServer/Communication/Packets/Processors/FootstepPacketProcessor.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Jannify <[email protected]>
I have tested the PR as of commit 7e56f40, so if code reviews are approving, then this is ready to merge imo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to update FootstepSounds_OnStep_PatchTest
(change the number of instructions added by the transpiler). Also please use "Remove and Sort usings" before committing changes to a file.
It works fine IG but for some reason, the local player also sends packet for exosuits even if they're not walking those.
Can we have some sort of check that the FootstepSounds is for sure emitted by the local player or by an exosuit controlled by the local player ?
Co-authored-by: tornac1234 <[email protected]>
Co-authored-by: tornac1234 <[email protected]>
Co-authored-by: tornac1234 <[email protected]>
Currently we're using a switch case to determine what asset is being played, and the exosuit has a different step sound so if I change the default case to be an early return and add a case for the land step sound, it would ensure that footstep packets are only sent for footsteps and not exosuit steps. I would also move that switch/case higher up so that it triggers before we get too far into the method. Would that be an acceptable solution to your suggestion @tornac1234 ? |
Co-authored-by: tornac1234 <[email protected]>
Co-authored-by: tornac1234 <[email protected]>
…bstraction Co-authored-by: Jannify <[email protected]>
Co-authored-by: Jannify <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM CW
Co-authored-by: gussandst <[email protected]> Co-authored-by: WerewolfsX <[email protected]>
Test went well, @tornac1234 needs to review again before merging though |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will be good for merge after this change (squash and merge)
Co-authored-by: tornac1234 <[email protected]>
Fixes issue #2164
Does what the title says